Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: remove unused stakater annotations and add cleanup #266

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

shreddedbacon
Copy link
Member

@shreddedbacon shreddedbacon commented Nov 20, 2023

This removes the old stakater annotations from ingress objects and replaces some existing labels and annotations with new ones, deprecating old ones..

In fitting with uselagoon/lagoon#3597 this would deprecate the label lagoon.sh/primaryIngress in favour of route.lagoon.sh/primary=true|false.

It will also replace the existing annotation monitor.stakater.com/overridePath with the new annotation route.lagoon.sh/path-override containing the same value.

The script contained within the build-deploy-docker-compose.sh could be used outside of a deployment, but only once the build-deploy-tool has received the update. This script could be used to fix ingress in environments if deployments are not able to be performed before other systems start to stop using the older labels/annotations.

It would be ideal that existing tooling or controllers that may use these old labels and annotations adopt the new ones, with a fallback to support the old ones for a period of time.

closes #149

Copy link

@domcyrus domcyrus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lagoon.sh/primaryIngress is already in use and set on many ingresses. Therefore if we want to change this annoatation we would need to have something like an upgrade path which changes the existing annotation to the new one route.lagoon.sh/primary and also update the ingress operator.

@shreddedbacon
Copy link
Member Author

shreddedbacon commented Nov 23, 2023

lagoon.sh/primaryIngress is already in use and set on many ingresses. Therefore if we want to change this annoatation we would need to have something like an upgrade path which changes the existing annotation to the new one route.lagoon.sh/primary and also update the ingress operator.

Thanks for the review. This is still a draft at the moment, and if we decide to move forward with the label or annotation changes, you will need to adjust the ingress operator to handle the new one, or both.

This part of the build handles replacing the old with the new. https://github.com/uselagoon/build-deploy-tool/pull/266/files#diff-6ed90425f159edcdd2d40254544ed7e5086e2c9f2f44dfbc00d04f7af038e2e6R1209

As the description of this PR states, there is also a way to go through and update clusters to change the label on all resources (the script can be easily tweaked to do an entire cluster), in which case the ingress operator would only need to know the new one.

Once we decide on the path forward, we will inform you of the change you need to perform.

@domcyrus
Copy link

@shreddedbacon I see that you do have an upgrade path but the issue is that the ingress operator is relying already on it. Therefore changing it to the new format without supporting the old one is not feasible as you would need to synchronize the roll out with all the potential pitfalls of doing this changes in sync.
The second issue is with our dashboards which do also make use of this annotations and which will break as well in a way which is not very easy to fix unfortunately because the data is already in prometheus with the old label and there is no way you can change the data in prometheus/thanos...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Primary ingress domains shoud be labled
2 participants